Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure docker images can build in pipeline #1208

Merged
merged 1 commit into from
Mar 4, 2025
Merged

Ensure docker images can build in pipeline #1208

merged 1 commit into from
Mar 4, 2025

Conversation

junlarsen
Copy link
Member

@junlarsen junlarsen commented Feb 25, 2025 •
edited
Loading

The docker build is now tested in CI

This introduces some internal changes, explained in review comments

@vercel Vercel
Copy link

vercel bot commented Feb 25, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
dashboard 🔄 Building (Inspect) Visit Preview Mar 4, 2025 5:05pm
web ✅ Ready (Inspect) Visit Preview Mar 4, 2025 5:05pm

@junlarsen Graphite App
Copy link
Member Author

junlarsen commented Feb 25, 2025 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@junlarsen junlarsen changed the title Build containers in CI Deploy container applications to staging Feb 25, 2025
@vercel vercel bot temporarily deployed to Preview – web February 25, 2025 14:20 Inactive
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch from 8f66d1c to 336f8b5 Compare March 3, 2025 16:13
@junlarsen junlarsen changed the title Deploy container applications to staging Ensure docker images can build in pipeline Mar 3, 2025
@junlarsen junlarsen changed the base branch from main to graphite-base/1208 March 4, 2025 15:31
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch from 336f8b5 to 8225c02 Compare March 4, 2025 15:31
@junlarsen junlarsen changed the base branch from graphite-base/1208 to users/junlarsen/rewrite_code_to_conform_to_erasablesyntaxonly March 4, 2025 15:32
This was referenced Mar 4, 2025
Merged
Merged
@vercel vercel bot temporarily deployed to Preview – web March 4, 2025 15:51 Inactive
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch 2 times, most recently from 5638784 to e85b7e6 Compare March 4, 2025 16:03
@junlarsen junlarsen force-pushed the users/junlarsen/rewrite_code_to_conform_to_erasablesyntaxonly branch from e6373c6 to d007dfe Compare March 4, 2025 16:11
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch 4 times, most recently from 496d673 to 0558a88 Compare March 4, 2025 16:22
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch from 0558a88 to 6684804 Compare March 4, 2025 16:26
@junlarsen junlarsen force-pushed the users/junlarsen/rewrite_code_to_conform_to_erasablesyntaxonly branch from c8f5ed0 to 639324b Compare March 4, 2025 16:28
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch from 6684804 to eeb1ee7 Compare March 4, 2025 16:28
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch from eeb1ee7 to 1fc2e0c Compare March 4, 2025 16:42
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch from 1fc2e0c to 900bec7 Compare March 4, 2025 16:54
Base automatically changed from users/junlarsen/rewrite_code_to_conform_to_erasablesyntaxonly to main March 4, 2025 16:55
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch from 900bec7 to 17387ff Compare March 4, 2025 16:58
@junlarsen junlarsen force-pushed the users/junlarsen/build_containers_in_ci branch from 17387ff to 00299da Compare March 4, 2025 17:00
@junlarsen junlarsen marked this pull request as ready for review March 4, 2025 17:01
junlarsen
junlarsen commented Mar 4, 2025
View reviewed changes
apps/rpc/runtime.mjs
import path from "node:path"
import { URL } from "node:url"

export async function resolve(specifier, context, nextResolve) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrites ../index to ../index.ts for node imports. This means things work well with both next's build system and node --strip-experimental-types

packages/db/package.json
"private": true,
"exports": {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed, so that types only imports schemas, and none of the actual Prisma client stuff. The prisma client breaks on Vercel (and shouldn't be used there for any reason either).

Same goes for the test harness

packages/db/prisma/schema.prisma
@@ -6,7 +6,7 @@ generator zod {
provider = "zod-prisma-types"
output = "../src/schemas"

useMultipleFiles = true
useMultipleFiles = false
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Node does not support directory import, and this codegenerator generates directory import when using multiple files.

packages/db/src/index.ts
Comment on lines -1 to +13
import { type Prisma, PrismaClient } from "@prisma/client"
import { createRequire } from "node:module"
import type { Prisma, PrismaClient } from "@prisma/client"
import type { DefaultArgs } from "@prisma/client/runtime/library"

export * as schemas from "./schemas"
const require = createRequire(import.meta.url)
const { Prisma: _Prisma, PrismaClient: _PrismaClient } = require("@prisma/client")
export type * from "@prisma/client"

export const PrismaRuntime = _Prisma
export const PrismaClientRuntime = _PrismaClient
export type DBClient = PrismaClient<Prisma.PrismaClientOptions, never, DefaultArgs>
export const createPrisma = (databaseUrl: string): DBClient =>
new PrismaClient({
new _PrismaClient({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prisma does not generate ESM module code, so we have to createRequire() and require it here. For this reason, the Prisma export cannot be both the client and namespace (like the generated client does).

Therefore we have to distinguish between Prisma namespace (re-exported from the client), and the runtime values.

Copy link
Member Author

@junlarsen junlarsen Mar 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means the Dockerfile build was broken in #1168 , but we never noticed because CI never built them. We'd have to do this at some point anyways

@junlarsen junlarsen requested a review from brage-andreas March 4, 2025 17:09
@junlarsen junlarsen merged commit 95bd78e into main Mar 4, 2025
7 checks passed
@junlarsen junlarsen deleted the users/junlarsen/build_containers_in_ci branch March 4, 2025 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brage-andreas brage-andreas brage-andreas approved these changes

@jotjern jotjern Awaiting requested review from jotjern jotjern is a code owner

@henrikskog henrikskog Awaiting requested review from henrikskog henrikskog is a code owner

@Terbau Terbau Awaiting requested review from Terbau Terbau is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@junlarsen @brage-andreas